Skip to content

Conversation

@cijothomas
Copy link
Member

Continuing this https://github.com/open-telemetry/opentelemetry-rust/pull/2573/files for ExportResult. Only touching Metrics, but this should be reused for all signals.

@cijothomas cijothomas requested a review from a team as a code owner February 4, 2025 00:15
@cijothomas
Copy link
Member Author

@scottgerring @lalitb My attempt at Export error for Metrics. To ease merge conflicts, maybe we should get just the Enum/Result alone in a small PR and get it merged, so we can work without conflicts.
Let me know if you want to split the Enum to an short separate PR to be reviewed first.

#[derive(Error, Debug)]
/// Errors that can occur during export.
/// TODO: This should be ExportError, but that can be done after rest of repo changes are done.
pub enum ExportErrorMetric {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After checking more, this enum would also need AlreadyShutDown variant. Which means we should be able to re-use the same enum for Export,Shutdown,ForceFlush.

@scottgerring @lalitb Let me know if you agree. I'll make a small PR, just adding the common enum first, and then divide&conquer using it in all places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make a small PR, just adding the common enum first, and then divide&conquer using it in all places.

Looks good to me.

@cijothomas
Copy link
Member Author

Closing in favor of #2604

@cijothomas cijothomas closed this Feb 4, 2025
@cijothomas cijothomas deleted the cijothomas/metric-export-error branch February 4, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants